fix: switch keychain to Data Protection to eliminate auth prompts by datlechin · Pull Request #328 · TableProApp/TablePro

datlechin · 2026-03-15T06:29:24Z

Summary

Add centralized KeychainHelper using the Data Protection keychain (kSecUseDataProtectionKeychain: true) per Apple TN3137, eliminating per-item ACL prompts that appeared on every table open
Revert kSecAttrAccessible to AfterFirstUnlock (was WhenUnlockedThisDeviceOnly), restoring background reconnection while screen is locked
Refactor ConnectionStorage, AIKeyStorage, and LicenseStorage to delegate all keychain operations to KeychainHelper
Add one-time migration from legacy file-based keychain to Data Protection keychain on app launch

Closes #326

Test plan

Launch app — no keychain authorization dialog should appear
Verify existing passwords still load (migration works)
Lock screen, wait for ConnectionHealthMonitor 30s ping — reconnection should succeed (AfterFirstUnlock)
Fresh install: save a connection with password, relaunch, verify password persists
Run KeychainHelperTests and KeychainAccessControlTests

Summary by CodeRabbit

New Features
- Centralized, data-protected secure storage for keys with a one-time legacy migration at startup.
Bug Fixes
- Keychain authorization prompt no longer appears on every table open.
Tests
- Added tests covering secure storage save/load/delete, overwrite behavior, missing keys, and migration flag.
Chores
- Changelog updated with migration/fix notes.

coderabbitai · 2026-03-15T06:29:41Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 880788b4-3c65-4fa6-ab9e-78fac7ae4356

📥 Commits

Reviewing files that changed from the base of the PR and between f8bc0fa and 67176a6.

📒 Files selected for processing (2)

TablePro/Core/Storage/KeychainHelper.swift
TablePro/Core/Storage/LicenseStorage.swift

📝 Walkthrough

Walkthrough

Centralizes Keychain access into a new KeychainHelper, updates storage classes to delegate keychain operations to it, adds a startup migration call to move legacy items into Data Protection-backed keychain storage, and adds tests and changelog entries documenting the change.

Changes

Cohort / File(s)	Summary
Keychain Helper `TablePro/Core/Storage/KeychainHelper.swift`	Added new singleton `KeychainHelper` with save/load/delete (Data Protection), string helpers, and `migrateFromLegacyKeychainIfNeeded()` to migrate legacy items.
Storage Layer Delegation `TablePro/Core/Storage/AIKeyStorage.swift`, `TablePro/Core/Storage/ConnectionStorage.swift`, `TablePro/Core/Storage/LicenseStorage.swift`	Replaced direct SecItem* usage with `KeychainHelper.shared` calls; removed manual query construction and Security imports; public APIs unchanged.
App Startup `TablePro/AppDelegate.swift`	Invoke `KeychainHelper.shared.migrateFromLegacyKeychainIfNeeded()` in `applicationDidFinishLaunching` for one-time migration.
Tests `TableProTests/Core/Storage/KeychainHelperTests.swift`, `TableProTests/Core/Storage/KeychainAccessControlTests.swift`	Added tests for KeychainHelper round-trip, delete, upsert, nonexistent keys, migration flag; updated access-control test to expect data-protection flag and AfterFirstUnlock constant.
Changelog `CHANGELOG.md`	Added Unreleased note that Keychain authorization prompt no longer appears on every table open.

Sequence Diagram(s)

sequenceDiagram
    participant App as App Startup (AppDelegate)
    participant Helper as KeychainHelper
    participant Legacy as Legacy Keychain
    participant DPKeychain as Data Protection Keychain

    App->>Helper: migrateFromLegacyKeychainIfNeeded()
    Helper->>Legacy: Query legacy items
    Legacy-->>Helper: Return items

    loop per legacy item
        Helper->>DPKeychain: Save item (kSecUseDataProtectionKeychain)
        DPKeychain-->>Helper: Success / Error
        alt Success
            Helper->>Legacy: Delete legacy item
            Legacy-->>Helper: Deleted
        end
        Helper->>Helper: Log progress
    end

    Helper->>Helper: Set migration flag in UserDefaults
    Helper-->>App: Migration complete

sequenceDiagram
    participant Storage as Storage class (AIKeyStorage / ConnectionStorage / LicenseStorage)
    participant Helper as KeychainHelper (singleton)
    participant DPKeychain as Data Protection Keychain

    rect rgba(100,150,200,0.5)
    Storage->>Helper: saveString(value, forKey:)
    Helper->>DPKeychain: Save item with Data Protection
    DPKeychain-->>Helper: Success / Error
    Helper-->>Storage: Bool / result
    end

    rect rgba(150,200,100,0.5)
    Storage->>Helper: loadString(forKey:)
    Helper->>DPKeychain: Query item with Data Protection flag
    DPKeychain-->>Helper: Data / not found
    Helper-->>Storage: String? result
    end

    rect rgba(200,150,100,0.5)
    Storage->>Helper: delete(key:)
    Helper->>DPKeychain: Delete item
    DPKeychain-->>Helper: Success / not found
    Helper-->>Storage: Void
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

feat: add SSH TOTP/two-factor authentication support #315: Modifies ConnectionStorage keychain usage (adds TOTP secret methods) and is likely related to the centralization of keychain access in this PR.

Poem

🐰 I hopped through keys at break of dawn,

Moved treasures safe so prompts are gone,
Data-protected, tucked in tight,
No more popups in the night,
A little hop—your app runs right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main change: switching keychain to Data Protection to eliminate authentication prompts.
Linked Issues check	✅ Passed	The PR successfully addresses issue `#326` by implementing Data Protection keychain and migration logic to eliminate repeated authentication prompts.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the keychain refactoring objective: new KeychainHelper, migration logic, storage refactoring, and related tests.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch fix/keychain-data-protection

📝 Coding Plan

Generate coding plan for human review comments

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

fix: switch keychain to Data Protection to eliminate auth prompts

f8bc0fa

fix: add kSecUseDataProtectionKeychain to SecItemUpdate search query

67176a6

datlechin merged commit 0bfb2c2 into main Mar 15, 2026
2 checks passed

datlechin deleted the fix/keychain-data-protection branch March 15, 2026 06:35

This was referenced Mar 18, 2026

feat: add optional iCloud Keychain sync for connection passwords #364

Merged

feat: add reusable SSH tunnel profiles #385

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: switch keychain to Data Protection to eliminate auth prompts#328

fix: switch keychain to Data Protection to eliminate auth prompts#328
datlechin merged 2 commits intomainfrom
fix/keychain-data-protection

datlechin commented Mar 15, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Mar 15, 2026 •

edited

Loading

Review failed

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

datlechin commented Mar 15, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Mar 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Poem

❌ Failed checks (1 warning)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

datlechin commented Mar 15, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Mar 15, 2026 •

edited

Loading